-
Notifications
You must be signed in to change notification settings - Fork 16
Fix breaking changes from databricks-ai-bridge 0.4.2 #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Ann Zhang <ann.zhang@databricks.com>
Signed-off-by: Ann Zhang <ann.zhang@databricks.com>
Signed-off-by: Ann Zhang <ann.zhang@databricks.com>
Signed-off-by: Ann Zhang <ann.zhang@databricks.com>
python-version: ["3.10"] | ||
version: | ||
- {ref: "databricks-ai-v0.4.0", name: "v0.4.0"} | ||
- {ref: "databricks-ai-v0.3.0", name: "v0.3.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openai
integration with the tests we are running doesn't exist for older version of databricks-ai-bridge
version: | ||
- {ref: "databricks-ai-v0.4.0", name: "v0.4.0"} | ||
- {ref: "databricks-ai-v0.3.0", name: "v0.3.0"} | ||
- {ref: "databricks-ai-v0.2.0", name: "v0.2.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests for vector search tool integration don't exist in 0.1.0
run: | | ||
pytest integrations/openai/tests/unit_tests/test_vector_search_retriever_tool.py::test_vector_search_retriever_tool_init | ||
|
||
|
||
llamaindex_test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llamaindex
is not published and thus has no cross-version tests rn
pip install integrations/langchain[dev] | ||
- name: Run tests | ||
run: | | ||
pytest integrations/langchain/tests/unit_tests/test_vector_search_retriever_tool.py::test_init |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only testing basic init functionality here, since the functionality of other parts has changed. This still does catch the backwards compatibility issues that Serena saw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a comment saying the same? this is really awesome work!
Signed-off-by: Ann Zhang <ann.zhang@databricks.com>
@@ -111,5 +111,8 @@ def make_document(row_index: int, score: float): | |||
) | |||
def test_parse_vector_search_response(retriever_schema, ignore_cols, docs_with_score): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we also add a test for what an old version of databricks-langchain would've done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm w/ one nit! thanks for doing this ann
What was changed?
databricks-ai-bridge==0.4.2
by providing defaultsHow is this tested?